tests: prevent leaked tarantool and panic on bad start#586
Merged
oleg-jukovec merged 1 commit intomasterfrom May 5, 2026
Merged
tests: prevent leaked tarantool and panic on bad start#586oleg-jukovec merged 1 commit intomasterfrom
oleg-jukovec merged 1 commit intomasterfrom
Conversation
6dba26a to
0b5c01f
Compare
oleg-jukovec
requested changes
May 5, 2026
Collaborator
oleg-jukovec
left a comment
There was a problem hiding this comment.
Extra dot in the commit message:
Part of #147. -> Part of #147
Two related issues from #147 made test failures painful to diagnose: 1. When a test panicked, the spawned tarantool process was left running and kept its TCP port bound, so the next run failed with "address already in use" instead of the original test failure. 2. The defer/require ordering used in many tests was inst, err := test_helpers.StartTarantool(opts) defer test_helpers.StopTarantoolWithCleanup(inst) require.NoError(t, err) When StartTarantool returned a nil instance on error (the connect failure path returns nil after stopping the half-started process), the deferred cleanup ran on a nil pointer and panicked, hiding the real "Unable to start Tarantool" failure under a nil-deref stack. Fix the first by setting Pdeathsig=SIGTERM on the tarantool command on Linux via a new test_helpers.commandKillOnExit helper, with a no-op fallback for other platforms (darwin has no Pdeathsig equivalent — see the comment in cmd_other.go). Fix the second by reordering every offending test site to assert StartTarantool succeeded before deferring the cleanup. This is the standard Go idiom and avoids growing a nil-guard inside the helper that would mask further StartTarantool bugs. Part of #147
0b5c01f to
fe5c21d
Compare
Collaborator
Fixed. |
oleg-jukovec
approved these changes
May 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two related issues from #147 made test failures painful to diagnose:
address already in useinstead of the original test failure.defer test_helpers.StopTarantoolWithCleanup(inst)placed before therequire.NoError(t, err)panicked with a nil-pointer dereference whenStartTarantoolreturned a nil instance on error, hiding the real "Unable to start Tarantool" failure.Changes were made:
test_helpers.commandKillOnExithelper: setsPdeathsig=SIGTERMon Linux so the tarantool child dies with the test process. Darwin gets a no-op fallback (noPdeathsigequivalent — seecmd_other.go).StartTarantoolsucceeded before deferring cleanup. Standard Go idiom, and avoids growing a nil-guard inside the helper that would mask furtherStartTarantoolbugs.Part of #147.